Skip to content

Implemented Stable Marriage in Ruby #534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 24, 2018
Merged

Implemented Stable Marriage in Ruby #534

merged 3 commits into from
Dec 24, 2018

Conversation

BjoernAkAManf
Copy link
Contributor

Code should be self explanatory

@Gathros Gathros added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) Hacktoberfest The label for all Hacktoberfest related things! labels Oct 24, 2018
Copy link
Contributor

@nic-hartley nic-hartley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! No dealbreakers, but a lot of things I think could be cleaned up. Nice usage of obscure standard functions, but I found a couple you missed :)

@BjoernAkAManf
Copy link
Contributor Author

Will look into your suggestions next week.

Thanks!

Copy link
Contributor Author

@BjoernAkAManf BjoernAkAManf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took your suggestions into account. Pretty much straight forward changes.

Copy link
Contributor

@nic-hartley nic-hartley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really do not consider the display code you had self-explanatory at all. Thank you for changing it.

My complaint with shuffle! wasn't its complexity (as you said, it's hardly unusual), just that it's meant for in-place operations and you're using it to chain operations. Thank you for changing that, too.

@leios
Copy link
Member

leios commented Dec 24, 2018

I am sorry, I totally missed this review. If @nic-hartley is happy, so am I!

@leios leios merged commit 3679668 into algorithm-archivists:master Dec 24, 2018
@BjoernAkAManf BjoernAkAManf deleted the stable_marriage_ruby branch December 25, 2018 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest The label for all Hacktoberfest related things! Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants